AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, renamed-only files with no content changes

Findings

🔶 Medium — Missing CHANGELOG entry for new feature

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new parentAssociation XML property, a new V2 rendering path, and new XML schema changes are behaviour/API changes that require a changelog entry under [Unreleased]. The [Unreleased] section is currently empty.
Fix: Run pnpm -w changelog from repo root, or manually add to CHANGELOG.md:

## [Unreleased]

### Added

- We added a `Parent association` property that enables infinite-depth lazy-loading tree hierarchies (Tree Node V2).

🔶 Medium — Direct mutation of TreeNodeV2DataItem.treeNodeState outside React state

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState is mutated directly on the object that lives inside treeData (owned by useIncrementalTreeData). The forceRender call masks this, but React's concurrent mode and StrictMode can replay effects, making stale-closure reads observe inconsistent state. Mutation on shared ref-backed objects is also invisible to useMemo/useCallback consumers and breaks diffing assumptions.
Fix: Expose a setter from useIncrementalTreeData (or useInfiniteTreeNodes) that updates the node state through setTreeData, ensuring React sees a new reference:

// in useIncrementalTreeData, expose:
const setNodeState = useCallback((nodeId: string, state: TreeNodeState) => {
    const node = nodesByIdRef.current.get(nodeId);
    if (node) {
        node.treeNodeState = state;
        setTreeData([...rootsRef.current]);
    }
}, []);

Then remove the forceRender pattern from TreeNodeV2.

🔶 Medium — No unit tests for any v2 code

File: src/components/v2/ (no __tests__ directory)
Problem: Three new non-trivial hooks (useIncrementalTreeData, useInfiniteTreeNode) and a new component (TreeNodeV2) have no unit tests. The incremental tree data logic (parent re-placement, config reset, removed-items detection) is complex enough that regressions will be hard to catch without isolated unit tests.
Fix: Add src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts at minimum. Key cases to cover:

• Root nodes appear when parentId is undefined
• Child nodes attach to correct parent after incremental load
• Config change resets the tree
• Self-referencing node (parentId === id) is treated as root

🔶 Medium — aria-expanded always set on leaf nodes in v2

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded is unconditionally rendered on every <li role="treeitem">, including leaf nodes (hasChildren === false). Per WAI-ARIA 1.2, aria-expanded must not be present on leaf tree items — screen readers will incorrectly announce them as collapsible.
Fix:

<li
    key={node.id}
    className="widget-tree-node-branch"
    role="treeitem"
    tabIndex={0}
    {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

⚠️ Low — Non-interactive <span> used as click target for tree header

File: src/components/v2/TreeNode.tsx line 23
Problem: The header is a <span onClick={...}>. Only nodes with children are visually "clickable" (via the widget-tree-node-branch-header-clickable class), but the onClick fires on all nodes. Keyboard users can focus the <li> but cannot activate the toggle via Enter/Space on the span. The v1 implementation already has a dedicated keyboard handler — v2 is missing this.
Note: This is consistent with v1's span-based header, but v2 should add keyboard handling before shipping.

⚠️ Low — getExpandedFilterItems dependency array is empty but reads mutable refs

File: src/components/v2/hooks/useInfiniteTreeNode.ts line 32–34
Problem: getExpandedFilterItems is memoised with [] deps but reads loadedParentsByIdRef.current and loadedChildsByIdRef.current inside. Since these are refs this is technically safe (ref.current is not a dep), but the empty array is misleading — linters may flag it in future ESLint configs. A comment clarifying the intent would help.

⚠️ Low — helpers.ts uses non-relative import path

File: src/components/v2/hooks/helpers.ts line 3
Problem: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a bare non-relative specifier. The other v2 files use "../../../../typings/TreeNodeProps". This may work because of a paths alias in tsconfig, but it's inconsistent and could break if the alias is removed.
Fix: Use the relative path: import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps";

Positives

• Excellent structural separation: existing v1 code was reorganised into components/v1/ without any behaviour change, making the diff easy to audit.
• The incremental tree data hook handles out-of-order delivery correctly (children arriving before parents are temporarily placed as roots and re-parented when the parent arrives).
• Self-referencing cycle guard (node.parentId !== node.id) prevents infinite loops on malformed data.
• CSS-only collapse via grid-template-rows: 0fr is a clean, animation-friendly approach that avoids DOM removal and preserves scroll position.
• useIncrementalTreeData correctly detects removed items and resets tree state, preventing stale nodes after datasource refreshes.
• E2E tests use aria roles and .mx-name-* selectors (preferred order), include proper afterEach session logout, and test the full lazy-load flow including grandchildren.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, pure rename/no-change files

Findings

🔶 Medium — CHANGELOG entry missing for new feature

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: The [Unreleased] section is empty. This PR introduces a new XML property (parentAssociation), a new v2 rendering path, and lazy-loading behavior — all of which are user-visible runtime changes that require a changelog entry.
Fix: Run pnpm -w changelog from the repo root, or add manually under [Unreleased]:

### Added
- We added a `parentAssociation` property that enables a v2 lazy-loading mode for infinite-depth hierarchies using a self-referencing association.

🔶 Medium — aria-expanded applied to leaf nodes; clicking a leaf sets it to true

File: src/components/v2/TreeNode.tsx lines 22, 91-103
Problem: aria-expanded={isExpanded} is unconditionally set on every <li role="treeitem">, including leaf nodes (hasChildren === false). The ARIA spec says aria-expanded must only appear on items that can be expanded — its presence on a leaf implies falsely that the item is expandable. Worse, because onClick is also attached unconditionally (line 29), clicking a leaf node mutates node.treeNodeState to EXPANDED and triggers forceRender, causing aria-expanded to flip to "true" on a non-expandable item.
Fix:

// In renderRecursiveNode — only set aria-expanded when the node has children
<li
  key={node.id}
  className="widget-tree-node-branch"
  role="treeitem"
  tabIndex={0}
  {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

And guard onNodeClick to no-op on leaf nodes:

const onNodeClick = useCallback((node: TreeNodeV2DataItem) => {
    if (node.children.length === 0) return;
    // ... rest of handler
}, [appendItems]);

🔶 Medium — Interactive header uses <span onClick> instead of <button>

File: src/components/v2/TreeNode.tsx lines 23-30
Problem: The expand/collapse trigger is a <span> with onClick. Keyboard users (Enter/Space) cannot activate it, and screen readers won't announce it as interactive. The frontend guidelines require replacing <div>/<span> click handlers with semantic elements.
Fix: Replace the header <span> with a <button>:

<button
  type="button"
  className={classNames("widget-tree-node-branch-header", {
      "widget-tree-node-branch-header-reversed": iconPlacement === "left",
      "widget-tree-node-branch-header-clickable": hasChildren
  })}
  id={`${node.id}TreeNodeBranchHeader`}
  onClick={() => onNodeClick(node)}
  aria-label={typeof node.title === "string" ? node.title : undefined}
>

If the existing v1 behaviour for non-expandable nodes must be preserved (no cursor pointer), apply the hasChildren check to conditionally render a <button> vs a presentational <span>.

🔶 Medium — No unit tests for v2 hooks

File: src/components/v2/hooks/ (no __tests__ directory)
Problem: useIncrementalTreeData contains non-trivial tree-building logic (parent re-placement, config-change reset, removed-ID detection, orphan adoption). useInfiniteTreeNode manages Mendix datasource filter composition. Neither has unit tests. The E2E tests verify the golden path but won't catch edge cases like circular self-references, nodes arriving out of order, or config resets.
Fix: Add src/components/v2/hooks/__tests__/useIncrementalTreeData.spec.ts covering at minimum:

• Root-only render on first pass
• Child adopted when parent arrives later
• Tree reset when configChanged
• Tree reset when removedIdsDetected

⚠️ Low — Non-relative import path in helpers.ts

File: src/components/v2/hooks/helpers.ts line 3
Problem: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a bare (root-alias) path. Every other file in v2 uses a relative path ("../../../../typings/TreeNodeProps"). This will likely resolve correctly via the TypeScript paths config but fails if the file is compiled by a tool that does not load tsconfig.json path aliases (e.g. Jest without moduleNameMapper).
Fix:

import { TreeNodeContainerProps } from "../../../../typings/TreeNodeProps";

⚠️ Low — Direct mutation of node.treeNodeState with forceRender pattern

File: src/components/v2/TreeNode.tsx lines 92-103
Problem: node.treeNodeState is mutated directly on the shared TreeNodeV2DataItem object (which lives inside useIncrementalTreeData's refs) and forceRender is called to trigger re-render. This works today but breaks React Strict Mode double-invocation guarantees, makes state changes invisible to any future MobX/context consumer, and creates a hidden coupling between the render component and the data hook's mutable internals.
Fix: Consider exposing a setNodeState(id, state) updater from useIncrementalTreeData, or at minimum document the mutation contract with a comment.

Positives

• The v1/v2 dispatch in the entry-point (if (props.parentAssociation)) is clean and keeps backward compatibility without touching any existing v1 code paths.
• Extracting TreeNodeState to common/TreeNodeState.ts and re-exporting from v1/TreeNode.tsx is a clean way to share the enum without breaking existing imports.
• The CSS grid 0fr/1fr collapse animation with will-change and min-height: 0 on grid children is a well-known, performant technique — no layout thrashing.
• E2E tests cover lazy loading, multi-root independent expansion, startExpanded, and collapse — good breadth for the main user journeys.
• useIncrementalTreeData correctly handles out-of-order arrivals (children before parents) and re-places orphan nodes when their parent is later added.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, renamed files with zero diff

Findings

🔶 Medium — No CHANGELOG.md entry for runtime behavior change

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new XML property (parentAssociation) and an entirely new v2 rendering path are added. This is a visible behavior and API change — a CHANGELOG entry is required per repo conventions.
Fix: Run pnpm -w changelog inside packages/pluggableWidgets/tree-node-web and add an entry under [Unreleased] describing the new parentAssociation lazy-loading mode.

🔶 Medium — Direct state mutation on TreeNodeV2DataItem triggers a forceRender hack

File: src/components/v2/TreeNode.tsx lines 659–666
Problem: node.treeNodeState is mutated directly on the data item (a plain object from the ref-backed tree), then forceRender(v => v + 1) forces a re-render. This bypasses React's state model — the mutable tree lives outside React state, so the [, forceRender] sentinel is the only thing signalling React that anything changed. If useIncrementalTreeData ever produces a new array reference (e.g. after a config change) while a node is expanded, the mutation is lost because treeData is re-derived from setTreeData([...rootsRef.current]). The expanded/collapsed state should either be tracked in a Map held in a separate useState/useReducer, or the data items should be treated as immutable and replaced on each toggle.
Fix:

// Option: maintain a separate expanded-state map in useState
const [expandedIds, setExpandedIds] = useState<Set<string>>(new Set());

const onNodeClick = useCallback((node: TreeNodeV2DataItem) => {
    setExpandedIds(prev => {
        const next = new Set(prev);
        if (next.has(node.id)) {
            next.delete(node.id);
        } else {
            next.add(node.id);
            appendItems(node.item, node.children.map(c => c.item));
        }
        return next;
    });
}, [appendItems]);

Then pass expandedIds into renderRecursiveNode and derive isExpanded from the set rather than from node.treeNodeState.

🔶 Medium — useInfiniteTreeNode useEffect missing dependency: startExpanded

File: src/components/v2/hooks/useInfiniteTreeNode.ts line 975
Problem: startExpanded is read in the useEffect body but is listed as a dependency — that part is fine. However initializedRef.current is used as a one-shot guard that never resets when startExpanded changes after mount. If the host page changes startExpanded dynamically, the guard prevents the filter from being re-applied. The startExpanded value used in the effect is the value captured when initializedRef was first set; changes are silently ignored.
Fix: Reset initializedRef.current when startExpanded changes, or move the initialization logic into a separate useMemo/effect keyed on startExpanded so toggling startExpanded re-runs the root filter.

🔶 Medium — Header span uses onClick without keyboard support

File: src/components/v2/TreeNode.tsx lines 591–597
Problem: The <span className="widget-tree-node-branch-header" … onClick={…}> is the interactive toggle for expand/collapse. A <span> does not fire click events on Enter/Space for keyboard users, and the tabIndex={0} is set on the <li role="treeitem"> rather than on the clickable element. Per the guidelines and WCAG 2.1, interactive controls should be <button> elements (or have role="button" + onKeyDown).
Fix:

<button
    type="button"
    className={classNames("widget-tree-node-branch-header", { ... })}
    onClick={() => onNodeClick(node)}
    aria-expanded={isExpanded}
>

Move aria-expanded from the <li> to the button (ARIA authoring practice for treegrid/tree recommends the expanding control carry aria-expanded).

⚠️ Low — appendItems early-return when parent already loaded skips pre-loading children

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 922–947
Problem: When loadedParentsByIdRef.current.has(parentId) is already true but children is empty or undefined, the function returns without calling datasource.setFilter. If a previously-expanded node is re-clicked and the component re-calls appendItems (e.g. after a datasource reload), the filter is never refreshed and children may disappear. This is a subtle edge case but could surface in startExpanded scenarios where all nodes are pre-loaded.

⚠️ Low — renderRecursiveNode is a plain function inside the module, not a component

File: src/components/v2/TreeNode.tsx lines 578–622
Problem: renderRecursiveNode is called as a function (renderRecursiveNode(node, …)) rather than rendered as a JSX component (<RecursiveNode node={node} … />). This means React cannot reconcile individual nodes — every call to forceRender unmounts and re-mounts the entire subtree. Define it as a proper component (function TreeNodeItem(…)) and render it with JSX so React can diff individual nodes.

⚠️ Low — No unit tests for v2 components

File: src/components/v2/ (no __tests__/ directory)
Problem: useIncrementalTreeData, useInfiniteTreeNode, and the TreeNodeV2 component have no unit tests. The incremental placement logic in useIncrementalTreeData (config-change reset, parent-not-yet-loaded case, cycle guard candidate.id !== nodeId) has enough branching to benefit from targeted unit tests with EditableValueBuilder/ListValueBuilder.

⚠️ Low — TreeNodePreviewProps.parentAssociation typed as string (should be string | null)

File: typings/TreeNodeProps.d.ts line 1037
Problem: The preview type for parentAssociation is string while the container type is ListReferenceValue | undefined. The editor config uses values.parentAssociation !== null as the guard, but string can never be null in TypeScript — the guard is always truthy if the field is "" (empty string). Check how Mendix Studio Pro supplies unset association previews (usually null) and align the type and guard accordingly.
Fix: Change to parentAssociation: string | null in TreeNodePreviewProps, or follow what other association preview props use in this codebase.

Positives

• Clean v1/v2 separation — all existing v1 files moved intact to components/v1/ with only import-path corrections; no v1 behaviour changed.
• TreeNodeState correctly extracted to components/common/ so both v1 and v2 share the same enum without circular imports.
• CSS-grid 0fr / 1fr collapse animation is a clean, accessible technique that avoids display: none (content stays in the DOM for screen readers).
• E2E tests use .mx-name-* selectors and ARIA roles consistently; afterEach session logout is present; no hardcoded waitForTimeout calls; toBeAttached used instead of toBeVisible for lazy-loaded content.
• useIncrementalTreeData correctly guards against cycles (candidate.id !== nodeId) and handles the "child arrives before parent" case by re-placing orphaned roots.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, snapshot PNGs

Findings

🔶 Medium — Missing CHANGELOG entry for new feature

File: CHANGELOG.md
Problem: [Unreleased] is empty. Adding parentAssociation is a new public XML property and behaviour — it requires a changelog entry per repo conventions.
Fix: Run pnpm -w changelog or manually add under [Unreleased]:

### Added
- We added the `parentAssociation` property to enable infinite-depth lazy-loaded tree hierarchies (V2 mode).

🔶 Medium — <span> used as interactive element — keyboard inaccessible

File: src/components/v2/TreeNode.tsx line 23
Problem: The node header is a <span onClick>. Keyboard users cannot activate it with Enter or Space. Screen readers also won't announce it as interactive. V1 uses a dedicated button/header element with keyboard handlers.
Fix: Replace with <button> (or keep <span> but add role="button", tabIndex={0}, and onKeyDown). The cleaner fix is a <button>:

<button
    type="button"
    className={classNames("widget-tree-node-branch-header", {
        "widget-tree-node-branch-header-reversed": iconPlacement === "left",
        "widget-tree-node-branch-header-clickable": hasChildren
    })}
    id={`${node.id}TreeNodeBranchHeader`}
    onClick={() => onNodeClick(node)}
>

🔶 Medium — aria-expanded set on leaf nodes

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded={isExpanded} is placed on every <li role="treeitem">, including leaf nodes (hasChildren === false). Per WAI-ARIA, aria-expanded must only be present on nodes that can be expanded; setting it on leaves is a spec violation and confuses screen readers.
Fix: Only render the attribute when the node has children:

<li
    key={node.id}
    className="widget-tree-node-branch"
    role="treeitem"
    tabIndex={0}
    {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

🔶 Medium — V2 missing all keyboard navigation

File: src/components/v2/TreeNode.tsx
Problem: V1 has useTreeNodeBranchKeyboardHandler and useTreeNodeFocusChangeHandler for full arrow-key / Enter / Escape navigation, and uses a roving tabIndex pattern (active node: 0, others: -1). V2 sets tabIndex={0} on every node and has no key handlers at all. This is a complete accessibility regression versus V1.
Fix: Port the accessibility hooks from v1/hooks/ (or move them to common/) and wire them into the V2 tree. At minimum, add onKeyDown to the header that maps Enter/Space to onNodeClick.

🔶 Medium — No unit tests for V2 code

File: src/components/v2/
Problem: Three substantial new modules (TreeNode.tsx, useIncrementalTreeData.ts, useInfiniteTreeNode.ts) have no unit tests. useIncrementalTreeData especially — which handles incremental insertion, re-parenting, config-change resets, and cycle guards — has complex logic that is hard to verify through E2E alone.
Fix: Add src/components/v2/__tests__/useIncrementalTreeData.spec.ts and TreeNode.spec.tsx covering at minimum: root-only initial render, child insertion, parent change, startExpanded path, and config-change reset.

🔶 Medium — Mendix association value read without status check

File: src/components/v2/hooks/helpers.ts lines 14, 20
Problem: parentAssociation?.get(item).value and config.headerCaption?.get(item).value are accessed directly without checking .status first. When the value is loading, .value is undefined, which is silently swallowed here — but it means nodes render with a fallback ID instead of their real title while loading, and parent associations are resolved as undefined (treated as roots) until the data arrives.
Fix:

export function getParentId(item: ObjectItem, parentAssociation: TreeNodeContainerProps["parentAssociation"]): string | undefined {
    const ref = parentAssociation?.get(item);
    if (ref?.status !== ValueStatus.Available) return undefined;
    return ref.value?.id ? getItemId(ref.value) : undefined;
}

export function getItemTitle(item: ObjectItem, config: TreeConfigRef): ReactNode {
    if (config.headerType === "text") {
        const caption = config.headerCaption?.get(item);
        return caption?.status === ValueStatus.Available ? caption.value : getItemId(item);
    }
    const content = config.headerContent?.get(item);
    return content ?? getItemId(item);
}

🔶 Medium — Direct mutation of shared state object

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState = TreeNodeState.COLLAPSED_WITH_CSS mutates an object that is part of the ref-backed tree structure shared across renders. This works today because of the forceRender call, but it bypasses React's state model — devtools won't show state changes, and this pattern will break under React Strict Mode or concurrent features.
Fix: Store treeNodeState separately (e.g. a Map<string, TreeNodeState> in a useRef updated via setTreeData) or call setTreeData(prev => [...prev]) after mutating — at minimum, don't rely on mutation + imperative re-render for correctness.

⚠️ Low — Non-relative import in helpers.ts

File: src/components/v2/hooks/helpers.ts line 3
Note: import { TreeNodeContainerProps } from "typings/TreeNodeProps" uses a non-relative path. All other files in the package use explicit relative paths (../../../../typings/TreeNodeProps). This only works if a paths alias is configured in tsconfig.json, which is uncommon in this repo. Verify it resolves in CI builds and consider aligning with the relative-path convention.

⚠️ Low — will-change in static CSS rule

File: src/components/v2/ui/TreeNodeV2.scss lines 6–8
Note: will-change: grid-template-rows, opacity is declared statically on .widget-tree-node-v2-body. This permanently promotes all tree-node body elements to compositor layers. For trees with many nodes, this increases GPU memory usage even when no animation is running. Consider applying will-change only during animation via a transitioning class, or removing it entirely (the transition still works without it).

⚠️ Low — E2E: toBeAttached used where toBeVisible is appropriate

File: e2e/TreeNode.spec.js lines 76–77
Note: toBeAttached only verifies the element is in the DOM, not that it is visible to the user. After lazy-loading grandchildren, the intent is to confirm the user can see them. Use toBeVisible() instead.

Positives

• Clean V1/V2 split — routing at the widget entry point with a simple props.parentAssociation guard is easy to follow and preserves backward compatibility entirely.
• useIncrementalTreeData correctly handles out-of-order arrival (children before parents are temporarily placed as roots and re-parented when the parent arrives) and self-reference cycles (node.parentId !== node.id guard).
• isConfigChanged uses reference equality on Mendix props, which is correct — Mendix gives the same reference when the config hasn't changed, so this avoids spurious full-tree resets.
• Moving TreeNodeState to components/common/ and re-exporting from v1/TreeNode is a clean refactor that makes it available to V2 without a circular dependency.
• E2E tests use aria-expanded assertions for collapse state rather than visibility — appropriate since V2 uses CSS-grid for animation (children remain in the DOM).
• Snapshot baselines are committed alongside the new tests.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, pure renames with no content change

Findings

🔶 Medium — CHANGELOG entry missing for new XML property and v2 behavior

File: packages/pluggableWidgets/tree-node-web/CHANGELOG.md
Problem: A new parentAssociation XML property was added and an entirely new rendering path (v2) introduced. These are runtime/XML/behavior changes and require a CHANGELOG entry under [Unreleased].
Fix: Run pnpm -w changelog or manually add to CHANGELOG.md:

## [Unreleased]

### Added

- We added a new `parentAssociation` property that enables infinite-depth hierarchies using a self-referencing association, powered by a new lazy-loading tree renderer (v2).

🔶 Medium — Direct mutation of node.treeNodeState then forceRender — MobX-style state mutation anti-pattern in plain React

File: src/components/v2/TreeNode.tsx lines 93, 98
Problem: node.treeNodeState is a property of a plain object held inside React state (treeData). Mutating it directly and then calling forceRender bypasses React's immutable state model. React's reconciler compares object references — the same reference is mutated, so React.memo or any memoization downstream cannot detect the change. This will also silently break if useIncrementalTreeData ever uses Object.is comparison.
Fix: Either produce a new node object (immutable update) or call setTreeData in useIncrementalTreeData with a shallow copy. A minimal immutable approach:

const onNodeClick = useCallback(
    (node: TreeNodeV2DataItem) => {
        node.treeNodeState =
            node.treeNodeState === TreeNodeState.EXPANDED
                ? TreeNodeState.COLLAPSED_WITH_CSS
                : TreeNodeState.EXPANDED;

        if (node.treeNodeState === TreeNodeState.EXPANDED) {
            appendItems(node.item, node.children.map(child => child.item));
        }
        // trigger a new array reference so React sees the change
        setTreeData(prev => [...prev]);
    },
    [appendItems, setTreeData]
);

This requires setTreeData to be returned from useIncrementalTreeData.

🔶 Medium — aria-expanded rendered on all <li role="treeitem"> nodes, including leaf nodes

File: src/components/v2/TreeNode.tsx line 22
Problem: aria-expanded={isExpanded} is always set on every <li>. According to WAI-ARIA, aria-expanded must only be present on items that can be expanded (i.e., items with children). Leaf nodes must have no aria-expanded attribute. Setting it to false on a leaf node incorrectly signals to screen readers that the node is collapsed but could be expanded.
Fix:

<li
    key={node.id}
    className="widget-tree-node-branch"
    role="treeitem"
    tabIndex={0}
    {...(hasChildren ? { "aria-expanded": isExpanded } : {})}
>

🔶 Medium — useEffect in useInfiniteTreeNode has stale-closure risk on startExpanded

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 69–93
Problem: startExpanded is captured at initialization time via initializedRef. However, if the parent's startExpanded prop changes after the initial render (which Mendix props can), the effect will not re-run the initialization path because initializedRef.current is already true. The initializedRef guard bypasses the deps array intent.
Fix: Accept that startExpanded is an init-only value by capturing it in its own ref, making the intent explicit and preventing confusion:

const startExpandedRef = useRef(startExpanded); // snapshot on mount
// then use startExpandedRef.current inside the effect

Remove startExpanded from the deps array since the ref value doesn't change. This self-documents that it is intentionally read once.

⚠️ Low — <span onClick> on the header instead of <button>

File: src/components/v2/TreeNode.tsx lines 23–36
Problem: The node header uses a <span> with onClick. This is not keyboard-accessible out of the box — it will not fire on Enter/Space unless keyboard handlers are added, and it lacks implicit button semantics for screen readers. The v1 implementation uses proper button semantics via keyboard hooks.
Note: This is a medium concern from an accessibility standpoint but given the tree already uses tabIndex={0} on the <li>, it may partially work. Still, a <button> or at minimum role="button" with onKeyDown should be used for the header span to match v1 behaviour and WCAG 2.2 AA.

⚠️ Low — parentAssociation is string in TreeNodePreviewProps but nullable check uses !== null

File: src/TreeNode.editorConfig.ts line 66 / typings/TreeNodeProps.d.ts line 50
Problem: In TreeNodePreviewProps, parentAssociation is typed as string (not string | null). Studio Pro passes an empty string "" for unset association properties. The check values.parentAssociation !== null will always be true (an empty string is not null), meaning showChildren is always true when parentAssociation is used. The correct check is values.parentAssociation !== "" (or just truthy !!values.parentAssociation).
Fix:

const showChildren = values.hasChildren || !!values.parentAssociation;

⚠️ Low — No unit tests for new v2 components

File: src/components/v2/
Note: useIncrementalTreeData, useInfiniteTreeNode, and TreeNode.tsx (v2) are new, non-trivial logic files with no corresponding spec files. Consider adding at minimum unit tests for useIncrementalTreeData covering: initial render with root nodes, incremental load of children, config reset on change, and the re-parenting logic when parent arrives after child.

Positives

• Clean v1/v2 split with backward-compatible routing in TreeNode.tsx — existing users are unaffected.
• useIncrementalTreeData correctly handles the out-of-order arrival case (children arriving before parent) by re-placing nodes when the parent is later inserted.
• CSS-only collapse animation using grid-template-rows: 0fr/1fr avoids JS height measurement and is a solid pattern.
• E2E tests use getByRole("treeitem") and .mx-name-* selectors — follows the preferred selector hierarchy from the review guidelines.
• TreeNodeState enum correctly extracted to common/ so both v1 and v2 share the same constants without duplication.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, pure rename/no-content-change files

Findings

🔶 Medium — Direct mutation of treeNodeState on ref-stored objects bypasses React's rendering model

File: src/components/v2/TreeNode.tsx lines 116–121
Problem: node.treeNodeState is mutated in place on objects that live inside refs managed by useIncrementalTreeData. React never sees a new object reference, so the tree is only re-rendered because of the manual forceRender counter hack. This pattern is fragile: concurrent rendering, StrictMode double-invocations, or any future refactor can silently break it. React state (or at minimum a MobX action) should own the expand/collapse state.
Fix: Move treeNodeState into useState / a Map<string, TreeNodeState> stored in React state, keyed by node ID, and derive isExpanded from that map during render. useIncrementalTreeData should deal only with structural tree data (parent/child relationships), not UI state.

🔶 Medium — useEffect in useInfiniteTreeNode.ts runs on every render when datasource reference changes

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 69–93
Problem: datasource is in the dependency array. Mendix ListValue objects are typically recreated on every parent render, so datasource is a new reference each render cycle. This means the effect runs repeatedly, clearing initializedRef on the first run and then immediately re-triggering the pre-load logic on subsequent runs — causing infinite filter updates.
Fix: Depend on datasource.status and datasource.items (stable primitives Mendix actually changes when data changes) rather than the whole datasource object, matching the existing V1 pattern at v1/Root.tsx:45.

🔶 Medium — No unit tests for the new v2 hooks and component

File: src/components/v2/
Problem: useIncrementalTreeData, useInfiniteTreeNode, and TreeNodeV2 have no corresponding __tests__/*.spec.tsx files. The incremental tree-building logic in useIncrementalTreeData has several tricky branches (re-parenting nodes, cycle prevention, partial-load ordering) that are exactly the kind of code that breaks silently and requires unit test coverage. E2E tests cover the golden path, but not edge cases.
Fix: Add src/components/v2/__tests__/useIncrementalTreeData.spec.ts covering at minimum: root-only load, child loaded before parent (out-of-order), parent change (re-parenting), and config change causing full reset.

⚠️ Low — key prop placed on <li> inside a plain function (not a component)

File: src/components/v2/TreeNode.tsx line 28
Note: renderRecursiveNode is a plain function, not a React component, so the key prop on the returned <li> has no effect — React uses key only on elements returned directly from a component's render or a .map() call at the call site. The caller at line 138–147 passes these elements directly into treeData.map(...), so React does see the key there. But if renderRecursiveNode is ever called outside of a .map(), or if a linter/reader tries to reason about it, the misplaced key is confusing. Move key to the .map() call sites, or convert to a proper React component.

⚠️ Low — Icon-only expand/collapse button uses <span onClick> instead of <button>

File: src/components/v2/TreeNode.tsx lines 44–52
Note: The icon container is a <span> with an onClick. For openNodeOn === "iconClick" this is the sole interactive target, making it keyboard-inaccessible (no focus, no Enter/Space handler). Use <button type="button"> with an aria-label (e.g. "Expand Electronics") for the icon-only click target. The same pattern exists in the parent header <span> at line 34 for headerClick mode.

⚠️ Low — getExpandedFilterItems useCallback has an empty dependency array despite reading mutable refs

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 32–34
Note: getExpandedFilterItems reads loadedParentsByIdRef.current and loadedChildsByIdRef.current. Refs are stable so the empty [] dep array is technically safe — but the lint rule react-hooks/exhaustive-deps will flag this. Consider inlining the function body into the callers or adding a // eslint-disable-next-line with a short explanation to prevent future confusion.

Positives

• Clean v1/v2 split: all existing V1 code was preserved exactly through renames, with zero behaviour changes, making the diff easy to audit.
• useIncrementalTreeData correctly guards against self-referencing cycles (node.parentId !== node.id) and handles out-of-order arrival (re-placing orphan nodes once their parent arrives).
• CSS-only collapse animation using grid-template-rows: 0fr is a modern, paint-efficient technique that avoids height: auto transition hacks.
• datasource.setFilter using Mendix filter builders (association, equals, literal, or) is the correct way to drive lazy loading — no direct DOM/fetch calls.
• E2E tests include afterEach session logout and waitForLoadState("networkidle") in beforeEach, matching the required structure exactly.
• CHANGELOG entry is present and follows Keep a Changelog format.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, snapshot PNGs (baselines committed ✅)

Findings

🔶 Medium — No unit tests for any V2 code

File: src/components/v2/ (no __tests__/ directory exists)
Problem: TreeNodeV2, useInfiniteTreeNode, useIncrementalTreeData, and helpers.ts introduce ~390 lines of new logic with zero unit test coverage. The incremental tree-building algorithm in useIncrementalTreeData contains non-trivial branching (re-placement, orphan promotion, config-change reset) that is hard to validate through E2E alone.
Fix: Add a src/components/v2/__tests__/ directory with at minimum:

• useIncrementalTreeData.spec.ts — cover root placement, child placement, orphan promotion, parent-change re-placement, configChanged reset
• useInfiniteTreeNode.spec.ts — cover initial filter setup, appendItems filter updates, startExpanded path
• Use ListValueBuilder, obj() from @mendix/widget-plugin-test-utils for Mendix mocks.

🔶 Medium — Direct mutation of treeNodeState on ref-held objects bypasses React's rendering model

File: src/components/v2/TreeNode.tsx lines 116, 121
Problem: node.treeNodeState is mutated directly on the object stored inside rootsRef/nodesByIdRef (managed by useIncrementalTreeData), then a counter state is incremented to force a re-render. This is an unofficial pattern that couples the V2 component to internal ref mutation side-effects. If useIncrementalTreeData ever calls setTreeData([...rootsRef.current]) after an items update, the mutation is overwritten — the collapse state is silently lost without any error.
Fix: Lift collapse state into a Map<string, TreeNodeState> held in the component, keyed by node ID, and pass it down to the recursive renderer. onNodeClick then calls setExpandedState(...) instead of mutating:

const [expandedState, setExpandedState] = useState<Map<string, TreeNodeState>>(new Map());

const onNodeClick = useCallback((node: TreeNodeV2DataItem) => {
    setExpandedState(prev => {
        const next = new Map(prev);
        const current = next.get(node.id) ?? node.treeNodeState;
        next.set(node.id, current === TreeNodeState.EXPANDED
            ? TreeNodeState.COLLAPSED_WITH_CSS
            : TreeNodeState.EXPANDED);
        return next;
    });
    if (node.treeNodeState !== TreeNodeState.EXPANDED) {
        appendItems(node.item, node.children.map(c => c.item));
    }
}, [appendItems]);

🔶 Medium — renderRecursiveNode is a plain function returning ReactElement, but receives key as a prop on the <li> — key is silently ignored

File: src/components/v2/TreeNode.tsx line 28
Problem: key={node.id} is set on the <li> inside renderRecursiveNode, but renderRecursiveNode is called as a function, not as a component. React only uses key when it appears on elements returned in an array from JSX. Since the recursive calls at lines 63–71 do not pass a key to renderRecursiveNode(child, ...), child <li> elements in the inner <ul> have no stable key — React will use index by default, causing incorrect reconciliation when children reorder.
Fix: Either promote renderRecursiveNode to a named component and render it with <RecursiveNode key={child.id} ...>, or move the key to the call site:

{node.children.map(child => (
    <Fragment key={child.id}>
        {renderRecursiveNode(child, renderHeaderIcon, iconPlacement, openNodeOn, onNodeClick, children)}
    </Fragment>
))}

⚠️ Low — useInfiniteTreeNode useEffect deps include datasource but datasource.items changes don't always trigger a new datasource reference

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 69–93
Problem: The useEffect that populates loadedParentsByIdRef and calls setFilter depends on [datasource, ...]. In Mendix, the datasource object reference is typically stable across renders; only datasource.items and datasource.status change. The pre-load logic on lines 73–80 will never fire after initialization because datasource itself doesn't change. This may cause the "pre-load children of roots" step to silently fail.
Fix: Add datasource.items and datasource.status to the effect deps alongside datasource:

}, [datasource, datasource.items, datasource.status, getDatasourceFilter, getExpandedFilterItems, startExpanded]);

⚠️ Low — PR description is empty; PR type checkboxes are all commented out

File: PR body
Note: None of the PR template sections were uncommented. The description, PR type, and "what should be tested" are all absent. This is especially important for a new feature that introduces an infinite-depth tree rendering path — reviewers and QA have no guidance on how to configure a Mendix test project for it.

⚠️ Low — isConfigChanged ignores startExpanded changes

File: src/components/v2/hooks/helpers.ts line 25
Note: isConfigChanged compares headerType, headerCaption, headerContent, and parentAssociation but not startExpanded. If startExpanded is toggled at runtime, existing nodes won't reset to the new default state. Consider including it in the comparison, or document why it is intentionally excluded.

Positives

• The V1 / V2 split is clean: all existing V1 code is preserved verbatim and the new code paths are fully isolated — zero regression risk for existing widget consumers.
• TreeNodeState enum was correctly extracted to common/ before being referenced by both versions.
• The incremental tree-data algorithm correctly guards against self-referential cycles (node.parentId !== node.id) and handles orphan promotion when a parent arrives after its children.
• E2E tests use aria-expanded for collapse assertions (CSS-only collapse) instead of visibility checks — exactly the right approach for the grid-based animation.
• Screenshot baselines for existing tests were updated and committed alongside the code change.
• CHANGELOG.md entry is present and well-written.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, screenshot PNG baselines

Findings

🔶 Medium — Direct mutation of treeNodeState on a ref-owned object bypasses React rendering model

File: src/components/v2/TreeNode.tsx lines 116, 121
Problem: node.treeNodeState is mutated directly on the TreeNodeV2DataItem that lives inside rootsRef in useIncrementalTreeData. Mutating a ref-held object (not state) and then calling forceRender is fragile: concurrent React renders may observe a torn intermediate state, and this pattern makes it impossible to roll back or batch updates. useIncrementalTreeData is responsible for the shape of the tree, yet its owned objects are silently modified from outside.
Fix: Thread expandedIds: Set<string> as derived state owned by TreeNodeV2, passed down as a prop or via context, and keep TreeNodeV2DataItem immutable. Alternatively, move the expand/collapse state into useIncrementalTreeData by exposing a toggleNode(id) action that calls setTreeData internally.

🔶 Medium — renderRecursiveNode is a plain function called in JSX — missing key on the returned <li>

File: src/components/v2/TreeNode.tsx line 28
Problem: The key={node.id} prop is placed on the <li> inside renderRecursiveNode, but the call site at line 63 (node.children.map(child => renderRecursiveNode(...))) returns a ReactElement from a plain function — not a component. React keys must be on the element returned directly from Array.prototype.map, and they work correctly here for the root-level treeData.map, but the children.map at line 63 also needs keys. Because renderRecursiveNode returns the <li key={node.id}>, this actually does work — but it creates a subtle footgun: if the function is ever wrapped in a fragment or extra element, the key would silently move to the wrong level. Converting renderRecursiveNode to a proper TreeNodeBranchV2 component would make key placement explicit and enable React.memo.

🔶 Medium — useInfiniteTreeNode.ts: appendItems logic skips filter update when parent is new and has no children

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 37–66
Problem: When appendItems(newItem) is called (no children argument), and loadedParentsByIdRef does not already contain the parent, the code reaches the else branch at line 61 and correctly adds the parent to loadedParentsByIdRef. However, when loadedParentsByIdRef already has the parent (line 41) but children is empty or undefined, the if (children && children.length > 0) check on line 42 is false — the function skips to datasource.setFilter (line 64) without moving the item from loadedChildsByIdRef to loadedParentsByIdRef. Re-expanding a previously collapsed node that has no further children never updates the filter unnecessarily — that's fine — but expanding a node for the first time that was pre-loaded as a child of another node (i.e. it is in loadedChildsByIdRef) will not be promoted to loadedParentsByIdRef, so its own children will never be included in the filter.
Fix:

// In the `if (loadedParentsByIdRef.current.has(parentId))` branch,
// promote from loadedChilds even when children array is absent/empty:
if (loadedChildsByIdRef.current.has(parentId)) {
    loadedParentsByIdRef.current.set(parentId, loadedChildsByIdRef.current.get(parentId)!);
    loadedChildsByIdRef.current.delete(parentId);
} else {
    loadedParentsByIdRef.current.set(parentId, newItem);
}
// then update filter whether or not `children` was provided
datasource.setFilter(getDatasourceFilter(getExpandedFilterItems()));

🔶 Medium — useIncrementalTreeData: config object identity changes on every render, causing the effect to fire every render

File: src/components/v2/TreeNode.tsx lines 101–110, src/components/v2/hooks/useIncrementalTreeData.ts line 138
Problem: treeConfig is built with useMemo in TreeNodeV2, which is correct. But useIncrementalTreeData's useEffect dep-array includes config as a whole object ([items, config]). Even though treeConfig is memoised, any re-render that produces a new object reference (e.g. if props.headerCaption itself is re-created by the Mendix runtime on each render) will re-run the full effect and rebuild the tree. The internal isConfigChanged comparison is already doing the right structural check — but it is called inside the effect after the fact, not as the dep-array guard.
Fix: Store individual config fields as separate stable refs, or split the effect into two — one for items (incremental) and one for config (full rebuild) — so a props identity churn on headerCaption doesn't unnecessarily trigger tree rebuilds.

⚠️ Low — editorConfig.ts: hasChildren property is still shown when parentAssociation is set, not hidden

File: src/TreeNode.editorConfig.ts lines 222–228
Problem: When parentAssociation has a value, the code calls hidePropertyIn(defaultProperties, values, "hasChildren"). However hasChildren is a boolean property that Studio Pro users may have previously configured; silently hiding it when V2 is active is correct, but the startExpanded and children props are still shown. In V2 mode, children (the slot) is passed through to children?.get(node.item) on line 61 of v2/TreeNode.tsx — so it is intentional — but consider hiding startExpanded-related sub-properties or at minimum adding a Problem validation note that warns the user when both parentAssociation and hasChildren have conflicting values.

⚠️ Low — PR template not filled in: type and description sections are empty

File: PR body
Problem: All ### Pull request type section options remain commented out, and the ### Description section is blank. This makes it impossible for reviewers or the release team to know what kind of change this is.
Fix: Uncomment the relevant <!-- New feature --> line and add a short description of what the V2 mode does and how to test it.

⚠️ Low — useInfiniteTreeNode.ts: comment typo loadedChilds → loadedChildren

File: src/components/v2/hooks/useInfiniteTreeNode.ts lines 16, 44
Note: Minor, but the variable is loadedChildsByIdRef and comments call the concept "loadedChilds" — should be "loadedChildren".

Positives

• Excellent structural decision to place shared types (TreeNodeState, HeaderIcon) in components/common/ before introducing v2 — avoids circular deps between v1 and v2.
• useIncrementalTreeData correctly handles out-of-order item arrival (child before parent) and cycle detection in the same pass — the test suite for this hook is thorough and tests each edge case independently.
• CSS-only collapse animation using grid-template-rows: 0fr is a modern, performant approach that avoids JS timers and works with prefers-reduced-motion natively if combined with a media query in a follow-up.
• The treeConfig useMemo in TreeNodeV2 is a good pattern for preventing unnecessary deep-tree rebuilds.
• E2E tests correctly use aria-expanded for collapsed state assertions rather than toBeVisible() — avoids false positives from CSS-only hide.
• afterEach session logout is present, preventing session limit exhaustion in CI.

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml, e2e/**-snapshots/

Findings

🔶 Medium — Keyboard users cannot expand/collapse v2 tree nodes

File: src/components/v2/TreeNode.tsx lines 34–52
Problem: The expand/collapse onClick handlers are attached to <span> elements, but the focusable element is the <li role="treeitem" tabIndex={0}>. When a keyboard user tabs to a tree item and presses Enter or Space, nothing happens — the click never fires on the span. The v1 implementation has useTreeNodeBranchKeyboardHandler to handle exactly this. V2 drops it entirely.

Fix: Add an onKeyDown handler to the <li> (or move the click handler there):

<li
    role="treeitem"
    tabIndex={0}
    aria-expanded={hasChildren ? isExpanded : undefined}
    onClick={hasChildren ? () => onNodeClick(node) : undefined}
    onKeyDown={hasChildren ? e => {
        if (e.key === "Enter" || e.key === " ") {
            e.preventDefault();
            onNodeClick(node);
        }
    } : undefined}
>

🔶 Medium — Direct mutation of ref-backed tree nodes triggers fragile re-render

File: src/components/v2/TreeNode.tsx lines 116–126
Problem: onNodeClick directly mutates node.treeNodeState on the object stored inside the useRef map in useIncrementalTreeData, then calls forceRender to trigger a re-render. This bypasses React's state model. In React 18 concurrent mode, an aborted render can read the mutated value before React intended, causing a torn state. The [, forceRender] = useState(0) pattern on line 82 is a code smell that signals state is living outside React.

Fix: Keep expand/collapse state in proper React state or manage it through useIncrementalTreeData's setTreeData. The cleanest fix is to return a setter from useIncrementalTreeData:

// In useIncrementalTreeData, expose a toggle function
const toggleNode = useCallback((nodeId: string, nextState: TreeNodeState) => {
    const node = nodesByIdRef.current.get(nodeId);
    if (node) {
        node.treeNodeState = nextState;
        setTreeData([...rootsRef.current]);
    }
}, []);

🔶 Medium — isConfigChanged ignores startExpanded, causing stale tree on prop change

File: src/components/v2/hooks/helpers.ts lines 25–35
Problem: isConfigChanged compares headerType, headerCaption, headerContent, and parentAssociation, but not startExpanded. If startExpanded changes after the initial render (e.g., driven by a Mendix expression), the tree will not rebuild and newly added nodes will use the stale initial state.

Fix:

return (
    previous.headerType !== next.headerType ||
    previous.headerCaption !== next.headerCaption ||
    previous.headerContent !== next.headerContent ||
    previous.parentAssociation !== next.parentAssociation ||
    previous.startExpanded !== next.startExpanded
);

⚠️ Low — MENDIX_VERSION=11.9.1 hardcoded in e2e scripts

File: package.json lines 28–29
Note: Pinning the Mendix version inline in the npm script means the repo-wide default is silently overridden for this package. If CI upgrades to a newer runtime, these scripts will stay on 11.9.1 without any warning. Consider externalising this via a .env.test file or a comment noting this is intentional and temporary.

⚠️ Low — PR description is entirely empty

File: PR body
Note: All template sections (PR type, description, test steps) are still commented out. Please uncomment the relevant type, add a description of what changed and why, and fill in the "What should be covered while testing?" section so reviewers and QA know how to validate the new parentAssociation feature.

Positives

• Solid test coverage for both new hooks: useIncrementalTreeData covers orphan re-parenting, cycle detection, out-of-order arrival, and config-change rebuilds — these are exactly the edge cases that matter for a lazy-loaded tree.
• The CSS-only collapse animation (grid-template-rows: 0fr → auto 1fr) is a clean, performant approach that avoids JS height measurement.
• Clean v1/v2 split: existing widget behaviour is preserved exactly; the new code is entirely additive behind the parentAssociation guard in TreeNode.tsx.
• isConfigChanged shallow-comparison guard in useIncrementalTreeData correctly prevents unnecessary full tree rebuilds on unrelated prop changes.
• CHANGELOG entry present and correctly describes the new capability.
• E2E tests use aria-expanded assertions rather than visibility checks for CSS-only collapse — correctly matches the implementation.